Description: Raw SQL string interpolation inserts host values directly into an INSERT statement without
sanitization, enabling SQL injection if site settings contain malicious input.
20150818190757_create_embeddable_hosts.rb [24-26]

records.each do |h|
  execute "INSERT INTO embeddable_hosts (host, category_id, created_at, updated_at) VALUES ('#{h}', #{category_id}, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)"
end

• Create ticket/issue

Follow the guide to enable codebase context checks.

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
SQL injection risk: User-controlled host values from site settings are interpolated directly into SQL INSERT
without parameterization, introducing potential SQL injection during migration.

records.each do |h|
  execute "INSERT INTO embeddable_hosts (host, category_id, created_at, updated_at) VALUES ('#{h}', #{category_id}, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)"
end

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing auditing: Creation, update, and deletion of embeddable hosts are not explicitly logged with user,
action, and outcome details, which may hinder audit trail requirements.

def create
  save_host(EmbeddableHost.new)
end

def update
  host = EmbeddableHost.where(id: params[:id]).first
  save_host(host)
end

def destroy
  host = EmbeddableHost.where(id: params[:id]).first
  host.destroy
  render json: success_json
end

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Edge case handling: The URL parsing in record_for_host returns false on invalid inputs but lacks
logging/context and may not handle uppercase or trailing-dot hosts comprehensively.

def self.record_for_host(host)
  uri = URI(host) rescue nil
  return false unless uri.present?

  host = uri.host
  return false unless host.present?

  where("lower(host) = ?", host).first
end

def self.host_allowed?(host)
  record_for_host(host).present?
end

Use a temporary ActiveRecord model for data insertion in the migration to
prevent a potential SQL injection vulnerability from raw SQL string
interpolation.

db/migrate/20150818190757_create_embeddable_hosts.rb [24-26]

-records.each do |h|
-  execute "INSERT INTO embeddable_hosts (host, category_id, created_at, updated_at) VALUES ('#{h}', #{category_id}, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)"
+class TempEmbeddableHost < ActiveRecord::Base
+  self.table_name = :embeddable_hosts
 end
 
+records.each do |h|
+  TempEmbeddableHost.create!(host: h, category_id: category_id)
+end
+

• Apply / Chat

The data hydration logic for _ids arrays in store.js.es6 is too broad and risks
causing unintended side effects. It should be refactored to be more specific or
opt-in for models that require it.

Examples:

      const m = /(.+)\_id(s?)$/.exec(k);
      if (m) {
        const subType = m[1];

        if (m[2]) {
          const hydrated = obj[k].map(function(id) {
            return self._lookupSubType(subType, type, id, root);
          });
          obj[self.pluralize(subType)] = hydrated || [];
          delete obj[k];

 ... (clipped 8 lines)

Solution Walkthrough:

Before:

// app/assets/javascripts/discourse/models/store.js.es6

_hydrateEmbedded(type, obj, root) {
  const self = this;
  Object.keys(obj).forEach(function(k) {
    // Regex only matches properties ending in `_id`
    const m = /(.+)\_id(s?)$/.exec(k);
    if (m) {
      if (m[2]) { // if it ends in `_ids`
        // ... logic to hydrate an array of IDs
        obj[self.pluralize(subType)] = hydrated || [];
      } else { // if it ends in `_id`
        // ... logic to hydrate a single ID
      }
    }
  });
}

After:

// app/assets/javascripts/discourse/models/store.js.es6

_hydrateEmbedded(type, obj, root) {
  const self = this;
  Object.keys(obj).forEach(function(k) {
    // Only hydrate `_ids` for models that have opted in
    if (/_ids$/.test(k) && self.modelFor(type).hasHydratableIds(k)) {
      // ... logic to hydrate an array of IDs
    } else if (/_id$/.test(k)) {
      // ... logic to hydrate a single ID
    }
  });
}

// In a model definition (e.g., embedding.js.es6)
export default Ember.Object.extend({
  // Opt-in mechanism
  hasHydratableIds(key) {
    return key === 'embeddable_host_ids';
  }
});

Add .filter(Boolean) after mapping embedded records to remove any null or
undefined values that could cause runtime errors.

app/assets/javascripts/discourse/models/store.js.es6 [196-202]

 if (m[2]) {
   const hydrated = obj[k].map(function(id) {
     return self._lookupSubType(subType, type, id, root);
-  });
+  }).filter(Boolean);
   obj[self.pluralize(subType)] = hydrated || [];
   delete obj[k];
 } else {

• Apply / Chat